Allow overriding CompletionHandler, InitHandler, WorkspaceDiagnosticsHandler - #3719

Since the plan is to keep having a single jdt.ls instance running, I don't see how those changes would help. Instead I think we should rather have a mechanism to register init, completion, [...whatever] participants, that would augment the existing jdt.ls behavior

Can one of the admins verify this patch?

Since the plan is to keep having a single jdt.ls instance running, I don't see how those changes would help.

The full set of changes so far is here: trancexpress@578383c

The entry point would be in the application, by extracting: JavaLanguageServerPlugin.startLanguageServer(this);

An example application that overrides the respective bits would then be:

public class DslLanguageServerApplication extends LanguageServerApplication {
	@Override
	protected void startLanguageServer() throws IOException {
    	JavaLanguageServerPlugin jlsp = JavaLanguageServerPlugin.getInstance();
		TelemetryManager telemetryManager = new TelemetryManager();
		boolean firstTimeInitialization = ProjectUtils.getAllProjects().length == 0;
		telemetryManager.onLanguageServerStart(System.currentTimeMillis(), firstTimeInitialization);
		BaseJDTLanguageServer ls = new DslLanguageServer(jlsp .getProjectsManager(), jlsp.getPreferencesManager(), telemetryManager);
		JavaLanguageServerPlugin.startLanguageServer(this, ls);
	}
}

Then vscode-java will have to start this application, we'll likely be patching this directly (as I don't see how to avoid patching vscode-java for some of the things we need).

Instead I think we should rather have a mechanism to register init, completion, [...whatever] participants, that would augment the existing jdt.ls behavior

Sounds great! What do you have in mind for the registration mechanism? A plain method, Eclipse extensions, something else?

For completion and diagnostics we would need some way to merge all completions/diagnostics?

For diagnostics I'm not sure how we communicate that existing diagnostics should be deleted, when there are multiple handlers. E.g. WorkspaceDiagnosticsHandler handles a deleted file, I'm not sure how that would look with multiple handlers (would all of them be checking for deletion, or the "new" ones rely on WorkspaceDiagnosticsHandler to do that?).

For the init handler we would need the ServerCapabilities object to adjust it.

@fbricon could you let me know what you have in mind?

Instead I think we should rather have a mechanism to register init, completion, [...whatever] participants, that would augment the existing jdt.ls behavior

Sounds great! What do you have in mind for the registration mechanism? A plain method, Eclipse extensions, something else?

I've pushed all the changes we need so far, to override handlers.

As well as prevent overriding JDT preferences, in our case we want the same preferences as our Eclipse application. We can also rely on a preference for this, it doesn't need to be a VM property.

Regardless of the registration mechanism for handlers, we would like to re-use as much code from JDT LS as possible. If we would just add registration mechanisms, we would need to copy a lot of code.

And then there are cases like the static CompletionHandler.selectedProposal, where if we would set/register a different completion proposal, we would need to set this field too.

Does the full set of changes make our intent/proposal more clear?

As mentioned above, the application we would start is:

public class DslLanguageServerApplication extends LanguageServerApplication {

    @Override
    public Object start(IApplicationContext context) throws Exception {
        return super.start(context);
    }

    @Override
	protected void startLanguageServer() throws IOException {
    	JavaLanguageServerPlugin jlsp = JavaLanguageServerPlugin.getInstance();
		TelemetryManager telemetryManager = new TelemetryManager();
		boolean firstTimeInitialization = ProjectUtils.getAllProjects().length == 0;
		telemetryManager.onLanguageServerStart(System.currentTimeMillis(), firstTimeInitialization);
    	BaseJDTLanguageServer ls = new DslLanguageServer(jlsp .getProjectsManager(), jlsp.getPreferencesManager(), telemetryManager);
    	JavaLanguageServerPlugin.startLanguageServer(this, ls);
	}
}

Bits are overridden with:

public class DslLanguageServer extends JDTLanguageServer {

	private final ProjectsManager pm;
	private final PreferenceManager preferenceManager;
	private final TelemetryManager telemetryManager;

	public DslLanguageServer(ProjectsManager projects, PreferenceManager preferenceManager, TelemetryManager telemetryManager) {
		super(projects, preferenceManager, telemetryManager);
		this.pm = projects;
		this.preferenceManager = preferenceManager;
		this.telemetryManager = telemetryManager;
	}


    @Override
    public CompletionHandler createCompletionHandler(PreferenceManager preferences) {
        return new DslCompletionHandler(preferences);
    }

    @Override
    public InitHandler createInitHandler() {
        return new DslInitHandler(pm, preferenceManager, client, WorkspaceExecuteCommandHandler.getInstance(), telemetryManager);
    }

    @Override
    public WorkspaceDiagnosticsHandler createWorkspaceDiagnosticsHandler() {
        return new DslWorkspaceDiagnosticsHandler(client, pm, preferenceManager.getClientPreferences(), getDocumentLifeCycleHandler());
    }
}

Thank you @angelozerr , I'll take a look!

Thank you @angelozerr , I'll take a look!

You are welcome! And if you like the idea, we are in discussion with Liberty Tools team to provide a dedicaced plugin which provides those lsp features extension point.

We'll continue with patching JDT LS until we have a more clear picture of what we need.

@trancexpress sorry I'm just back from PTO today.

So essentially what jdt.ls would allow, would be an Eclipse extension points to contribute participants, somewhat similar to #3732

Alright, thank you @fbricon! Once we have implemented the required features, we'll likely try to add such extensions.

If I find time earlier, I'll also try just for the handlers here - since we definitely need them.